-
-
Notifications
You must be signed in to change notification settings - Fork 282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MBS-13117: Prevent localizing admin-only messages #3091
Conversation
c6f2654
to
73d3fce
Compare
po/Makefile
Outdated
test_no_admin: $(ADMINPERL) $(ADMINTT) $(ADMINJS) $(wildcard extract_pot_templates) | ||
@echo "Testing that no admin files contain any translatable messages (MBS-13117)"; | ||
@count=0; \ | ||
for file in $(ADMINPERL); do \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth moving test_no_admin
to its own bash script in order to avoid all the newline escapes?
(Not sure specifying $(ADMINPERL) $(ADMINTT) $(ADMINJS)
in the Makefile as dependencies is strictly needed, since this rule is only invoked once by the tests.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, moved in commit 6e0619e
6e0619e
to
465c4cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems good with the caveat that assuming we merge #2480 first, which would seem the most sensible order, it will need some rebasing to "un-delete" some of the translation calls for the files that are no longer admin only (any forms would still be admin-only IIRC but since they won't be under /admin maybe they should use l_admin
instead of nothing).
Part of MBS-13117
There is no non-module file **.pl under lib/ anyway, but even if there was any it would be for admin only.
Implement all eventually needed functions to prevent translating messages destinated to admin in both admin and non-admin pages, as part of MBS-13117.
Prevent translating messages destinated to admin in non-admin pages, as part of MBS-13117.
It is equivalent to expand2react(cleanMsgid) but it makes it explicit that it is intentionally to not translate messages destinated to admin, as part of MBS-13117.
To prevent any further accidental localization in admin pages, this patch makes CI tests to exit with error with a non-empty list of lines calling localization functions (l, ln, lp, N_l, N_ln, N_lp).
There will never be any context needed for not translatable messages.
465c4cd
to
7e2e74e
Compare
Just rebased on latest |
280947f
to
2311fd9
Compare
The column labels for languages and scripts were exposed to admins only but still translated. These messages would be removed from translations after the previous commit 05960f8 from the following files: - root/admin/attributes/Attribute.js - root/admin/attributes/Language.js - root/admin/attributes/Script.js However those will likely be needed again after resolving MBS-11178. This patch preserves column labels and their translations by temporarily defining these in a unused file until MBS-11178 gets resolved.
2311fd9
to
e76ec04
Compare
Problem MBS-13117
Messages intended for admins only are needlessly extracted for translation.
Indeed admins are required to be English-speaking anyway.
So it is a waste of time for translators so far.
Solution
((t)exp).(N_)l[np]_admin
functions for admin-only messages in other pages.It makes explicit that some messages are intentionally not translated because these are intended for admin.
This approach works well in JavaScript, should we follow the same in Perl (under
lib
only)?Draft progress checklist
po/mb_server_pot
after the singular/plural issue has been resolved.Action